Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[issue_tracker] Fixes some of the functionality of the module #4690

Closed

Conversation

maltheism
Copy link
Member

Brief summary of changes

Fixes a lot of issue_tracker bugs and the module isn't functional in current state. I'm just publishing this PR right now because it solves a lot of the bugs for geting the module close to working.

Some of the issues: Checkbox not working, Site not being populated in the table, clicking an issue not populating site and I can't remember everything because it was frustrating to debug & fix.

This resolves issue...

To test this change...

  • todo

@maltheism maltheism added Category: Bug PR or issue that aims to report or fix a bug Area: UI PR or issue related to the user interface 21.0.0 Testing labels May 23, 2019
@maltheism
Copy link
Member Author

maltheism commented May 23, 2019

Please don't review just yet.
edit: Okay ready to review.

Copy link
Contributor

@zaliqarosli zaliqarosli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please remove package-lock.json and any console.logs

@maltheism
Copy link
Member Author

Hey @zaliqarosli, there isn't any console.logs and I thought we now commit the package-lock.json file. I'm looking at the differences between mine vs the old and it seems that https is now used in mine which is actually better.

@@ -273,6 +274,13 @@ class IssueForm extends Component {
$.ajax(this.props.DataURL, {
dataType: 'json',
success: function(data) {
console.log(data);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

console.log!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zaliqarosli hah missed it thanks :)

@zaliqarosli
Copy link
Contributor

@maltheism yeahh unsure about the package-lock. i forgot what we decided after that update PR. @johnsaigle do you remember?

@maltheism
Copy link
Member Author

@zaliqarosli #4408

@johnsaigle
Copy link
Contributor

My preference would be to submit updates to .lock files in their own PRs. If you attach them to other PRs it can be a little harder to go back and track changes and discussions.

I don't really deal much with the JS side though so I think you should just go with whatever works best for you all.

@maltheism
Copy link
Member Author

maltheism commented May 29, 2019

Hey @zaliqarosli & @johnsaigle my opinion is we just commit the package-lock.json and literally ignore it in PRs. Reason is every time I run make or npm run compile the file is made and when I go to add files I'm just doing git add * if I'm having a happy day with my workflow. Having to dodge the file by adding all files modified manually is a joke of a life and shouldn't happen much anyway. I think the reason why my package-lock.json using https compared to the old is because I'm using the latest npm and all the other developers never update their toolchain.
Anyway right before publishing release we could checkout 21.0-release, make a pr that just runs npm run compile and commit the package-lock.json file. Otherwise it would be nice to just ignore the file. Whatever the solution happens to be.. I want my git add * command or I'll be more sad in life.

@zaliqarosli
Copy link
Contributor

still one more error left on the 'Create New Issue' page

Screenshot 2019-05-30 10 22 19

@zaliqarosli
Copy link
Contributor

module seems to be working fine now, need to do a proper testing. whatever the solution is for package-lock.json, i don't think it should be included in this PR so i'm still requesting for it to be removed from this PR. if you want to use git add * functionality, you can also easily do git checkout -- package-lock.json after. i personally don't use git add * so this is not any issue i'm invested in, sorry!

@maltheism
Copy link
Member Author

Hey @zaliqarosli I'm leaving the package-lock.json in this PR.

@driusan
Copy link
Collaborator

driusan commented Jun 3, 2019

Please split this into multiple PRs.. it can't really be reviewed right now, because it doesn't describe what it's fixing, and it's going to conflict with other PRs that are fixing individual problems with the issue tracker.

@zaliqarosli zaliqarosli added the State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) label Jun 6, 2019
@zaliqarosli zaliqarosli added the State: Needs work PR awaiting additional work by the author to proceed label Jun 6, 2019
@zaliqarosli
Copy link
Contributor

you can remove the package-lock.json file from this PR now because #4791 takes care of that

@driusan
Copy link
Collaborator

driusan commented Jun 12, 2019

As far as I can tell, everything that this PR was trying to do has been fixed in other more targeted PRs now.

@driusan driusan closed this Jun 12, 2019
@maltheism maltheism deleted the 21.0-release_watching_fix branch May 24, 2020 02:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: UI PR or issue related to the user interface Category: Bug PR or issue that aims to report or fix a bug State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) State: Needs work PR awaiting additional work by the author to proceed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants